fix: avoid generating massive functions in get_mtkparameters_reconstructor#4539
Merged
Conversation
oscardssmith
approved these changes
May 15, 2026
Contributor
|
Good to go? |
Member
Author
|
No, pretty significant test failures. Some I can't easily reconcile. |
AayushSabharwal
force-pushed
the
as/faster-iprobpmap
branch
from
0120442 to
7f84f75
Compare
Contributor
|
Which ones? It is hard to get an overview when a lot of tests typically fail on master as well. What is expected to pass / fail? |
Member
Author
|
(1, ModelingToolkit/InterfaceII) should only fail with BVP-related stuff. QA should fail. |
Contributor
|
Claude said something like: So perhaps do a conversion instad of a type assert, like: diff --git a/lib/ModelingToolkitBase/src/systems/problem_utils.jl b/lib/ModelingToolkitBase/src/systems/problem_utils.jl
index 22c0bcb3..03ea9f2d 100644
--- a/lib/ModelingToolkitBase/src/systems/problem_utils.jl
+++ b/lib/ModelingToolkitBase/src/systems/problem_utils.jl
@@ -878,25 +878,25 @@ function (recon::MTKParametersReconstructor)(src, dst)
tunablevals = recon.tunables_fn(src)
else
tunable_elT = promote_type(eltype(dst_ps.tunable), eltype(src_ps.tunable))
+ tunablevals = recon.tunables_fn(src)
if ArrayInterface.ismutable(dst_ps.tunable)
tunable_T = Base.promote_op(similar, typeof(dst_ps.tunable), Type{tunable_elT})
- tunablevals = recon.tunables_fn(src)::tunable_T
else
tunable_T = StaticArraysCore.similar_type(typeof(dst_ps.tunable), tunable_elT)
- tunablevals = recon.tunables_fn(src)::tunable_T
end
+ tunablevals = convert(tunable_T, tunablevals)::tunable_T
end
if dst_ps.initials isa SVector{0}
initialvals = recon.initials_fn(src)
else
initial_elT = promote_type(eltype(dst_ps.initials), eltype(src_ps.initials))
+ initialvals = recon.initials_fn(src)
if ArrayInterface.ismutable(dst_ps.initials)
initial_T = Base.promote_op(similar, typeof(dst_ps.initials), Type{initial_elT})
- initialvals = recon.initials_fn(src)::initial_T
else
initial_T = StaticArraysCore.similar_type(typeof(dst_ps.initials), initial_elT)
- initialvals = recon.initials_fn(src)::initial_T
end
+ initialvals = convert(initial_T, initialvals)::initial_T
end
nonnumerics = recon.nonnumerics_fn(src)maybe? |
Member
Author
|
No, the |
AayushSabharwal
force-pushed
the
as/faster-iprobpmap
branch
3 times, most recently
from
3df9ec1 to
bd2fb43
Compare
Member
Author
|
SciMLSensitivity still doesn't like the typeasserts :( |
AayushSabharwal
force-pushed
the
as/faster-iprobpmap
branch
2 times, most recently
from
8380ab4 to
1fa9629
Compare
…ructor` This used to be the lion's share of the compile time in large models. The infrastructure added here might be useful in other places too.
Enables supporting ReverseDiff AD
AayushSabharwal
force-pushed
the
as/faster-iprobpmap
branch
from
1fa9629 to
fcdcfe5
Compare
AayushSabharwal
force-pushed
the
as/faster-iprobpmap
branch
from
fcdcfe5 to
1f026bd
Compare
Contributor
|
yay, happy to see this merged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This used to be the lion's share of the compile time in large models. The infrastructure added here might be useful in other places too, as a way to build simple one-off observed functions that mostly access
u/p.